Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SD-NOTIFY proxy in conmon #7607

Closed

Conversation

goochjj
Copy link
Contributor

@goochjj goochjj commented Sep 11, 2020

Requires containers/conmon#182

Sets up bind mounts for a sd-notify socket, and passes it to the env of the OCI runtime. NOTIFY_SOCKET is removed from the environment so the OCI runtime does not process the socket, conmon proxies it instead.

Discussion #6688 #7316

@goochjj
Copy link
Contributor Author

goochjj commented Sep 11, 2020

@haircommander @edsantiago @rhatdan PTAL

This is goochjj#1 rebased against master

I can squash if you'd like.

@goochjj goochjj force-pushed the libpod-sd-notify-with-conmon branch 2 times, most recently from c0b0ab9 to 7406483 Compare September 11, 2020 16:45
@goochjj
Copy link
Contributor Author

goochjj commented Sep 11, 2020

Tests are likely failing because of conmon dependency.

@rhatdan
Copy link
Member

rhatdan commented Sep 11, 2020

@cevich @haircommander We need an updated conmon to test with.

@haircommander
Copy link
Collaborator

pr has to be merged first :)

@cevich
Copy link
Member

cevich commented Sep 15, 2020

pr has to be merged first :)

It's possible to hijack the new image build process, since it all happens inside a PR (merging isn't necessary). It can also be run manually/locally...but the docs PR isn't merged yet.. Though this is probably "the hard way" for anyone who hasn't done it. Merge the change (here) first, then do the tests in a separate PR is the lazy/easy way to do this.

@mheon
Copy link
Member

mheon commented Oct 7, 2020

There's a chance we dragged in a newer Conmon as part of the new CI changes from @cevich - a rebase might make this pass?

@cevich
Copy link
Member

cevich commented Oct 8, 2020

a rebase might make this pass?

I'm showing conmon-2.0.21-2.fc32-x86_64 if using the images currently set on master. Is a newer version than that needed?

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: goochjj
To complete the pull request process, please assign vrothberg after the PR has been reviewed.
You can assign the PR to them by writing /assign @vrothberg in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member

rhatdan commented Oct 27, 2020

@haircommander PTAL

@haircommander
Copy link
Collaborator

LGTM

@rhatdan
Copy link
Member

rhatdan commented Oct 27, 2020

@goochjj Could you rebase. @haircommander Do we have the support in conmon at this point?

@haircommander
Copy link
Collaborator

haircommander commented Oct 27, 2020

yes though I don't know if the VMs have the correct version

@mheon
Copy link
Member

mheon commented Oct 27, 2020

@haircommander What is the minimum version requirement?

This leverages conmon's ability to proxy the SD-NOTIFY socket.
This prevents locking caused by OCI runtime blocking, waiting for
SD-NOTIFY messages, and instead passes the messages directly up
to the host.

Signed-off-by: Joseph Gooch <mrwizard@dok.org>
@goochjj goochjj force-pushed the libpod-sd-notify-with-conmon branch from 39f627a to 7c4a4c4 Compare October 28, 2020 16:17
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Nov 29, 2020

Replaced by #8508

@rhatdan rhatdan closed this Nov 29, 2020
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants